Skip to content

Conversation

@sahithiRavindranath
Copy link

@sahithiRavindranath sahithiRavindranath commented Oct 13, 2025

changes for vfio device access permission change.

@sahithiRavindranath sahithiRavindranath force-pushed the spyre1 branch 2 times, most recently from 9d8c958 to 7096b9a Compare October 14, 2025 10:16
@sahithiRavindranath sahithiRavindranath force-pushed the spyre1 branch 3 times, most recently from c02023f to d894b47 Compare October 23, 2025 09:58
try:
ret = True
if os.stat(full_path).st_gid != gid:
ret = False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checks all files under vfio_dir, whereas the check immediately below
if stat.S_ISCHR(mode): looks only for character device files inside
vfio_dir.

May I know why we are checking files other than character devices?

Even if their group ID does not match the expected value, those files will not be
added to the per_check object.

We can address this in one of two ways:

  1. If there is a need to check all files under vfio_dir, remove the
    character device check so that both the group ID and ownership checks are
    performed for all files.

  2. Else keep the group ID check within the if stat.S_ISCHR(mode): condition.

from servicereportpkg.check import FilesCheck
from servicereportpkg.utils import is_package_installed
from servicereportpkg.check import ConfigurationFileCheck
from servicereportpkg.utils import is_read_write_to_all_users
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is not longer used. So lets remove the import. More info info the comment below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say you can even remove is_read_write_to_all_users function.

mode = os.stat(file_path).st_mode
return (
not(bool(mode & stat.S_IROTH) and # Read permission for others
bool(mode & stat.S_IWOTH)) and # Write permission for others
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also white space expected between not nad (. also things are not aligned properly.
Something like this:

    return (
        not (bool(mode & stat.S_IROTH) and  # Read permission for others
             bool(mode & stat.S_IWOTH)) and  # Write permission for others
        bool(mode & stat.S_IRUSR) and  # Read permission for owner
        bool(mode & stat.S_IWUSR) and  # Write permission for owner
        bool(mode & stat.S_IRGRP) and  # Read permission for group
        bool(mode & stat.S_IWGRP)      # Write permission for group
    )

Changed the vfio device access permission such that only root and
group users will have access to the device.

Signed-off-by: Sahithi Ravindranath <[email protected]>
@sourabhjains
Copy link
Contributor

Merge the patch to spyre branch:
19ac37d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants